Skip to content

Fix CUDA-aware MPI synchronization in PGemm - #7688

Merged
mohanchen merged 1 commit into
deepmodeling:developfrom
Stardust0831:fix/pgemm-cuda-mpi-sync
Jul 27, 2026
Merged

Fix CUDA-aware MPI synchronization in PGemm#7688
mohanchen merged 1 commit into
deepmodeling:developfrom
Stardust0831:fix/pgemm-cuda-mpi-sync

Conversation

@Stardust0831

@Stardust0831 Stardust0831 commented Jul 24, 2026

Copy link
Copy Markdown

Reminder

  • I have read AGENTS.md and docs/developers_guide/agent_governance.md.
  • I have linked an issue or explained why this PR does not need one.
  • I have added adequate unit tests and/or case tests, or explained why not.
  • I have listed the exact verification commands run and their results.
  • I have described user-visible behavior changes, including INPUT parameter changes.
  • I have explained core-module impact for ESolver, HSolver, ElecState, Hamilt, Operator, Psi, or other source/ changes.
  • I have requested any needed governance exception below.

Linked Issue

Related to #7665. That PR introduced the SAI multi-GPU validation which exposed this failure and deliberately kept the existing numerical tolerance and reference unchanged. No separate issue is linked because this PR is the focused follow-up to that recorded CI failure.

Unit Tests and/or Case Tests for my changes

The race requires CUDA-aware MPI and multiple physical GPUs, so a CPU-only unit test cannot reproduce it. The existing 16_SDFT_GPU/005_PW_SDFT_MALL_BPCG_GPU case is used as the regression test without changing its input, tolerance, or reference.

Commands/checks run:

python3 tools/03_code_analysis/agent_governance_check.py \
    --base origin/develop --head HEAD --format text
git diff --check origin/develop..HEAD

# The SAI workflow invokes this form for the target case on 2 V100 GPUs:
bash tests/integrate/Autotest.sh -a <candidate-abacus> -n 2 -o 8 \
    -f CASES.task.txt -r '^005_PW_SDFT_MALL_BPCG_GPU$'

gh workflow run sai-gpu-full.yml \
    --repo Stardust0831/abacus-develop \
    --ref develop \
    -f source_sha=1a47d8e8f2ab06be2e240f69cf8d79325bf04bd4 \
    -f run_namespace=pgemm-sync-mode3

Before/after results from the same SAI GPU workflow and toolchain:

Source SAI run Target case Full GPU matrix
3178e9415ba411e97ef4c8e03c905cf0eab6b5f1 (before) 30087100482 FAIL, 2 GPUs, Slurm 703869_5 47 passed, 1 failed, 0 infrastructure failures
1a47d8e8f2ab06be2e240f69cf8d79325bf04bd4 (final PR head) 30151521504 PASS, 2 GPUs, Slurm 708786_5 48 passed, 0 failed, 0 infrastructure failures

The final-head run rebuilt ABACUS and passed the full 48-case GPU matrix. Its overall workflow conclusion is failure because the separate 2-node cuSolverMp smoke fixture, tests/15_rtTDDFT_GPU/19_NO_Si48_CUSOLVERMP_TDDFT_GPU, currently exists only in the still-open #7665 and is absent from upstream develop; that smoke exited before launching mpirun. This is independent of the PGemm path. An earlier composition run containing the same source fix together with the #7665 test infrastructure, 30114759440 at fc9a4bd0005a5cc2c4a0e093bc846a8c74a2d34a, passed all 48 GPU cases and the 2-node/16-GPU cuSolverMp smoke.

Focused diagnosis on SAI established the synchronization boundary:

  • 1 GPU with bndpar=1: deterministic and within tolerance.
  • CPU with bndpar=2: deterministic and within tolerance.
  • 2 GPUs with bndpar=2: nondeterministic energy deviations of about 1.35e-6 to 3.88e-6 eV before the fix.
  • 2 GPUs with CUDA_LAUNCH_BLOCKING=1: deterministic and within tolerance, indicating an asynchronous CUDA/MPI handoff race.
  • Patched 2-GPU binary with CUDA_LAUNCH_BLOCKING unset: 9/9 focused runs passed. Energy deviation was about 4.16e-9 eV, force deviation was zero, and stress deviation was 1.1e-5, below the existing 1e-7, 1e-4, and 1e-3 thresholds.
  • Same-allocation A/B timing medians were 4.69 s before and 4.57 s after across six measured runs per variant. Run-to-run noise was larger than the difference, so no measurable end-to-end regression was observed.

The final PR head passed the SAI candidate build, all 48 GPU cases, governance check, and diff check. The separate composition run passed the 2-node cuSolverMp validation as noted above. No local CUDA build was run because this workspace has no CUDA compiler/build tree; the final candidate was instead rebuilt from its immutable commit on the target SAI toolchain.

What's changed?

PGemmCN::multiply_col() passes the GPU-resident matrix A directly to CUDA-aware MPI_Isend. CUDA work producing A is asynchronous, so MPI could begin reading the buffer before the default CUDA stream finished writing it. The timing-dependent read caused the multi-GPU SDFT numerical deviation.

This PR synchronizes the default CUDA stream once immediately before each group of nonblocking MPI sends: before multiply_col() sends GPU-resident A in modes 1/2, and before multiply_row() sends GPU-resident B in mode 3. The synchronization is compiled only for CUDA-aware MPI builds, and only the GPU specialization performs work. It is intentionally placed once before all sends rather than once per destination, preserving overlap among the MPI_Isend requests and avoiding repeated synchronization.

Governance Notes

  • INPUT/docs changes: none. No INPUT parameter, public API, reference value, tolerance, or documented user workflow changes.
  • Core module impact: limited to the CUDA-aware MPI path in source_base PGemmCN::multiply_col(). CPU and non-CUDA-aware MPI paths are unchanged.
  • Test-path rationale: no new case is added because the existing SDFT GPU case is the exact modes 1/2 reproducer and now passes unchanged on the target multi-GPU environment. Mode 3 currently has no production call site or GPU integration case; its matching synchronization closes the same direct GPU-buffer-to-MPI handoff in the supported interface, while the final SAI build verifies that path compiles in the CUDA-aware MPI configuration.
  • Exceptions requested: none.

@Stardust0831
Stardust0831 force-pushed the fix/pgemm-cuda-mpi-sync branch from fc9a4bd to 7db8b74 Compare July 24, 2026 18:09
@QuantumMisaka QuantumMisaka added GPU & DCU & HPC GPU and DCU and HPC related any issues Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS labels Jul 25, 2026

@AsTonyshment AsTonyshment left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good fix! The SAI multi-GPU validation successfully exposed latent issues that previous CPU and single-GPU CI could not cover.

Comment thread source/source_base/para_gemm.cpp
@Stardust0831
Stardust0831 force-pushed the fix/pgemm-cuda-mpi-sync branch from 7db8b74 to 1a47d8e Compare July 25, 2026 08:40

@AsTonyshment AsTonyshment left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ZhouXY-PKU ZhouXY-PKU left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! Thanks for your contribution!

@mohanchen
mohanchen merged commit 18a0da0 into deepmodeling:develop Jul 27, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS GPU & DCU & HPC GPU and DCU and HPC related any issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants